-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: Mermaid pan & zoom #295
base: master
Are you sure you want to change the base?
Conversation
Hey @mjbvz, can I get a review on this PR? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this! Sorry it took me so long to test it
The panning and zooming seems to work well. However to start I think it should be presented differently. Let's do:
-
Add a new vscode configuration setting that lets you enable/disable pan zoom entirely. It can default to on, but there needs to be a way to turn the feature off entirely
-
Only show the pan/zoom controls when you hover over the diagram
-
Potentially add a command or right click option on the diagram that overrides the VS Code configuration. So if I have pan zoom enabled in my settings, I'd like the command to be able to turn it off in case it's causing any issues
A few other points I noticed:
-
The colors of the controls seem off, especially on dark themes. If you can, reuse some of the vscode theme colors. These should all be available inside of the markdown preview webview
-
The diagram get cut off. I'd try to get it match what GitHub does instead where the diagram is centered and takes up more horizontal space
I will take a more detailed look once some of those points are addressed. Let me know if you have any questions about these
Hey @mjbvz , thanks for getting back to me! I just slightly confused and want to clarify a few things:
or these controls after pan/zoom is enabled
|
Sure, following up:
|
Hey @mjbvz, sorry haven't been able to work on it much been kinda busy with other stuff. I've made a few changes according to some of your comments Toggle and Controls being too prominent
Color theme issues
Diagram being cut offSo for this issue I'm assuming you were talking about the diagram not taking the full width of the page when pan zoom is enabled. The reason for this was because I wanted the switching between pan zoom to look seamless, so when it gets turned on it takes the current size dimension of the diagram. But given it doesn't use the full width of the page would look like it's cut off. I've updated so that it does take the full width of the page when pan zoom is enabled, but the height is still the height of the current diagram so when you resize the width down it does look a bit skinny and tall which is something that we have to accept I think because the diagram needs a set height when pan zoom is enabled. |
This PR adds pan & zoom functionality in mermaid MD preview. There's a previous PR that attempted to add this functionality but had some issues. This PR tries address the issues mentioned in the comments.
#125
The changes include:
throw error
when a diagram fails to parse. From what I can see when mermaid library fails to parse the contents, we would catch the error and return a error message content. But for some reason it makes another throw error, so when you have multiple diagrams and one breaks it breaks everything else.after remove
throw error